egressgw: Let the EGW manager relax rp_filter on egress device#7
Open
MitchLewis930 wants to merge 1 commit intopr_047_beforefrom
Open
egressgw: Let the EGW manager relax rp_filter on egress device#7MitchLewis930 wants to merge 1 commit intopr_047_beforefrom
MitchLewis930 wants to merge 1 commit intopr_047_beforefrom
Conversation
Pods running on the Egress GW node fail to communicate with an external endpoint through the Egress GW due to the rp_filter in an environment where egress IP is assigned to a different interface than the one with the default route. The reply packets from the external endpoints are dropped by the rp_filter - A request from a local pod hits eth0 with the default route. It matches an IEGP, gets masqueraded & bpf-redirected to eth1 with Egress IP. - Replies hit eth1, are revSNATed, and passed on to the stack. rp-filter complains that they are received on eth1, when the route doesn't point towards eth1. This PR fixes this issue by relaxing rp_filter on interfaces with Egress IP. Signed-off-by: Yusuke Suzuki <yusuke.suzuki@isovalent.com>
There was a problem hiding this comment.
Pull request overview
This PR implements automatic relaxation of rp_filter settings on egress gateway interfaces to ensure proper packet routing for SNAT traffic.
Changes:
- Extended
gatewayConfigto track interface names and whether the local node acts as an egress gateway - Modified the manager to automatically set
rp_filter=2on egress interfaces when the node is configured as a gateway - Refactored
TestForIfaceWithIPv4Addressto return the interface name instead of just testing for existence
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/egressgateway/policy.go | Added interface name tracking and local gateway flag to gatewayConfig |
| pkg/egressgateway/manager.go | Implemented relaxRPFilter method to apply sysctl settings during reconciliation |
| pkg/datapath/linux/netdevice/netdevice.go | Refactored to expose interface name retrieval alongside existence check |
| pkg/egressgateway/manager_privileged_test.go | Added test coverage for rp_filter settings and helper functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+141
to
+142
| gwc.localNodeConfiguredAsGateway = false | ||
|
|
There was a problem hiding this comment.
Setting localNodeConfiguredAsGateway to false at the start of deriveFromPolicyGatewayConfig and then unconditionally setting it to true at line 175 means it will always be true when the function succeeds. This initialization appears unnecessary since the value is always overwritten before return. Consider removing the initial false assignment or documenting why this pattern is needed.
Suggested change
| gwc.localNodeConfiguredAsGateway = false |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR_047